Skip to content

feat(env_filter): support no_std#404

Open
cagatay-y wants to merge 2 commits into
rust-cli:mainfrom
cagatay-y:feature/filter-no_std
Open

feat(env_filter): support no_std#404
cagatay-y wants to merge 2 commits into
rust-cli:mainfrom
cagatay-y:feature/filter-no_std

Conversation

@cagatay-y
Copy link
Copy Markdown

Based on #378, this PR adds a new default feature std that allows the crate to be used in no_std crates. I tried to incorporate the feedback that was given to the previous PR. Hopefully, I didn't miss anything.

The PR is still a breaking change for crates that have the default features disabled and use Builder::from_env, or expect Builder::parse to print warnings to stderr and don't have the log target set to stderr.

It may make sense to squash 9c70eaf and 663c9c2 (and potentially b754c66). I kept them separate to make sure I don't misattribute the work of the author of the base PR.

Co-authored-by: WolverinDEV <git@did.science>
@cagatay-y cagatay-y changed the title refactor(env_filter): fix unreachable pub warning feat(env_filter): support no_std Apr 2, 2026
Comment thread crates/env_filter/src/lib.rs
Comment thread .github/workflows/ci.yml Outdated
@epage
Copy link
Copy Markdown
Contributor

epage commented Apr 2, 2026

FYI I expect PRs to be presented for how they should be reviewed and merged and not for how they were developed. Please clean up the commit history

@cagatay-y cagatay-y force-pushed the feature/filter-no_std branch from b754c66 to 364726a Compare April 6, 2026 12:00
Comment thread crates/env_filter/src/parser.rs Outdated
@cagatay-y cagatay-y requested a review from epage April 22, 2026 12:11
@cagatay-y
Copy link
Copy Markdown
Author

@epage, could you take another look at the PR?

Comment thread .github/workflows/ci.yml Outdated
Comment on lines +113 to +114
#[cfg(not(feature = "std"))]
log::warn!("{error}, ignoring it");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for creating the logger in the first place, so I don't think it is too meaningful to log at this point.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least in my use case, the filter member of the logger is a OnceCell<env_filter::Filter> and until the filter is fully initialised, the logger is returned a Option<&Filter>::None when trying to get it. When it receives the None, it simply lets the log message through, so the user is able to see the warning.

Comment thread crates/env_filter/src/lib.rs Outdated
Comment thread crates/env_filter/Cargo.toml
@cagatay-y cagatay-y force-pushed the feature/filter-no_std branch from 364726a to 2a7ae94 Compare May 26, 2026 13:51
@cagatay-y cagatay-y requested a review from epage May 26, 2026 13:55
Co-authored-by: WolverinDEV <git@did.science>
@cagatay-y cagatay-y force-pushed the feature/filter-no_std branch from 2a7ae94 to 8e062c3 Compare May 28, 2026 15:28
@cagatay-y
Copy link
Copy Markdown
Author

In the latest force-push, in addition to what was raised in the review, I fixed an error I made related to the handling of the features of the log dependency in the first version of the PR, and removed the imports for the tests which should have been removed with the change to always have std enabled for the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants